Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add title type #397

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open

Add title type #397

wants to merge 2 commits into from

Conversation

brookewp
Copy link
Contributor

After seeing the missing name field in #396, I thought it might make sense to add a 'title' type to the schema to set explicit titles. In DataViews, there are three special fields:

titleField: The id of the field representing the title of the record.
mediaField: The id of the field representing the media of the record.
descriptionField: The id of the field representing the description of the record.

When these are set, they get unique styling, which we are utilizing for image_url in the mediaField, which can be seen in the screenshots below.

This way, we don't have to guess like we do for block patterns. However, I left the check under string for sources like Airtable and Google Sheets, where we aren't explicitly defining each field and its type.

Before After
Screenshot 2025-02-27 at 12 52 37 PM Screenshot 2025-02-27 at 12 54 31 PM
Before After
Screenshot 2025-02-27 at 12 55 01 PM Screenshot 2025-02-27 at 12 54 39 PM

Signed-off-by: brookewp <[email protected]>
Copy link

Test this PR in WordPress Playground.

@brookewp brookewp requested a review from ingeniumed February 27, 2025 21:28
@alecgeatches
Copy link
Contributor

This is great, and would definitely help with auto-generation! What do you think about string:title or something like that? Like a type with hinting instead of a separate type. We could extend the same hints for other types like id:primary, image_url:primary, string:description, etc. I like the idea of adding information to the type rather than adding a different one, but it's subjective which is preferable.

Comment on lines 120 to 121
return sanitize_text_field( strval( $value ) );

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return sanitize_text_field( strval( $value ) );

$bindings['paragraphs'][] = [
'content' => [ $field, $name ],
];
break;

case 'title':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should remove the "autodetect" above and encourage use of this type?

Copy link
Member

@chriszarate chriszarate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dig it too.

What do you think about string:title or something like that? Like a type with hinting instead of a separate type. We could extend the same hints for other types like id:primary, image_url:primary, string:description, etc.

I like encoded it in the type as well. The only danger there is that you limit yourself in the future by tying it to another primitive type. For example, maybe we decided later that you wanted to indicate a level alongside the title, e.g. 'title' => [ 'level' => 'h1', 'content' => 'My title' ]

I pushed up a small fix in 311223f to supply the missing type definition and to close a loophole that allowed you to provide invalid types to the Validator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants